Skip to content

Fix E816 malformed diffpatch output issue#287

Merged
nickjvandyke merged 10 commits into
nickjvandyke:mainfrom
zawedcvg:main
Jun 12, 2026
Merged

Fix E816 malformed diffpatch output issue#287
nickjvandyke merged 10 commits into
nickjvandyke:mainfrom
zawedcvg:main

Conversation

@zawedcvg

@zawedcvg zawedcvg commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Tasks

  • I read CONTRIBUTING.md
  • I ensured my changes pass automated checks
  • I reviewed and understand the AI-generated code in my changes (if any)
  • I addressed actionable AI review feedback (if any)

Description

Issue basically was because of some incorrect file path handling. The current solution does work, but might not be the best, so let me know. Below is what the logs captured

2026-06-06 22:39:41 [opencode edits] opened edit diff buffer
{
  buf_name = "/home/user/.config/nvim/.config/nvim/lua/plugins/opencode.nvim/lua/opencode/events/permissions/edits.lua",
  buftype = "",
  current_buf = 38,
  current_tabpage = 2,
  current_win = 1038,
  error = "vim/_core/editor.lua:0: nvim_exec2(), line 1: Vim(diffpatch):E816: Cannot read patch output",
  filetype = "lua",
  modified = false,
  ok = false,
  windows_in_tab = { {
      buf = 38,
      diff = false,
      name = "/home/user/.config/nvim/.config/nvim/lua/plugins/opencode.nvim/lua/opencode/events/permissions/edits.lua",
      win = 1038
    } }
}

Testing

Related Issue(s)

Screenshots/Videos

@nickjvandyke

nickjvandyke commented Jun 7, 2026

Copy link
Copy Markdown
Owner

Thanks for taking a crack at this!

I'm a bit confused - why would this affect the diffpatch error? filepath is only used to open the current version of the edited file in tabnew. Whereas diffpatch receives patch_filepath, untouched by this PR.

@zawedcvg

zawedcvg commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

I could be wrong but I think its due to this from the :help diffpatch

Note that {patchfile} should only contain a diff for one file,
the current file. If {patchfile} contains diffs for other
files as well, the results are unpredictable.

I think because its trying to diff with a file that doesn't exist, the error is being thrown. At least this is my guess for now.

@zawedcvg

zawedcvg commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Ok you can reproduce the E816 pretty easily. have a file and generate a random patch for it and store it somewhere. Then open a non existing file and diffpatch with this patch. You get E816.

Also interestingly, if you open diffpatch from an existing file, but the file is not the correct one, you get this, which is a thing that sometimes occurs with opencode
image

@nickjvandyke

Copy link
Copy Markdown
Owner

Ah I see, thanks for explaining! So basically OpenCode might send a filepath that's incorrect relative to how Neovim searches for the file to open it? And resolving it as an absolute path fixes that?

Why do we sometimes make it relative to the home directory? I don't see when the absolute path would fail and need to fallback to that.

Also interestingly, if you open diffpatch from an existing file, but the file is not the correct one, you get this, which is a thing that sometimes occurs with opencode

Ah thanks for figuring that out! It puzzled me. Does this change fix it too?

@nickjvandyke

Copy link
Copy Markdown
Owner

Given we have the server available in this scope, I wonder if local filepath = server.cwd .. event.properties.metadata.diff would be even more reliable? I presume OpenCode always sends the filepath relative to its CWD. Whereas Neovim could very rarely resolve the absolute path differently with very particular directory structures and CWD mismatches.

@zawedcvg

zawedcvg commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

from what I remember the home directory branch was added because just making it absolute didn't completely solve. I might be misremembering but

local filepath = server.cwd .. event.properties.metadata.diff

doesn't solve it either. It has the same path issue because open code doesn't create paths wrt server.cwd.

Ah thanks for figuring that out! It puzzled me. Does this change fix it too?

Nope, it doesn't, just the malformed issue. I will spend sometime to see if I can find a better fix to the malformed issue and also fix the single line issue. Though the latter is a bit confusing in the sense that it opens the correct file, but the patch is not for that file? It doesn't make complete sense to me but I suspect it to be a similar file path issue. Sucks that diff patch doesn't have the greatest error handling but what can you do.

@zawedcvg

Copy link
Copy Markdown
Contributor Author

for the e816 error, I think this is the fix for now. Opencode sometimes sends the full path and sometimes everything but home, so I dont think there is a better solution.

I managed to figure out the bug with the single line thing, bad news is that there's nothing that can be done about it. So turns out that what opencode exposes in the diff, is a pretty printed version of the actual diff. It uses it just for display. It calls trimDiff on the actual unified patch, which can cause the patch to be malformed. Basically anytime the edit diff actually works with the plugin, it is due to luck. So yeah nothing can be done about this from the plugin side. I will open an issue and see if the opencode devs want to expose the actual patch, since it doesn't serve any purpose except for niche usecases like this.

@nickjvandyke

Copy link
Copy Markdown
Owner

/review

@github-actions

Copy link
Copy Markdown
Contributor

I've reviewed the changes. Here's my assessment:


What the code does: Resolves filepath by first trying :p (absolute from CWD), then falling back to $HOME + filepath. This prevents diffpatch from receiving a patch for a file path Neovim can't locate.

Correctness: The logic is sound — fnamemodify(":p") correctly handles already-absolute paths (returns as-is), and the HOME fallback handles the case where opencode sends paths like lua/opencode/events/... relative to $HOME. No bugs in the implementation.

Nit — silent failure when neither strategy works: If neither absolute_filepath nor home_filepath is readable, the code falls through with the original unresolved filepath. tabnew would create an empty "new file" buffer, and diffpatch would then fail with E816 all the same. Consider adding an early return with a vim.notify when the file can't be found, so the user gets a clear message instead of a cryptic E816.

Style — extraction: The resolution logic could be pulled into a small helper function for readability, e.g.:

local function resolve_filepath(filepath)
  local abs = vim.fn.fnamemodify(filepath, ":p")
  if vim.fn.filereadable(abs) == 1 then return vim.fn.fnameescape(abs) end
  local home = vim.fs.normalize(vim.fs.joinpath(vim.env.HOME or "", filepath))
  if vim.env.HOME and vim.fn.filereadable(home) == 1 then return vim.fn.fnameescape(home) end
  return nil
end

But this is purely stylistic — the current code works fine and is only 8 lines.

PR health: Good size (+14/-1), single file, focused on one issue. The discussion shows good collaboration between maintainer and contributor. The description could be stronger (clearer "why") but the conversation fills in the gaps.

New%20session%20-%202026-06-10T13%3A25%3A29.213Z
opencode session  |  github run

@zawedcvg

Copy link
Copy Markdown
Contributor Author

addressed the comments

Comment thread lua/opencode/events/permissions/edits.lua Outdated
Comment thread lua/opencode/events/permissions/edits.lua Outdated
@nickjvandyke

Copy link
Copy Markdown
Owner

/review

@github-actions

Copy link
Copy Markdown
Contributor

Review of PR: Fix E816 malformed diffpatch output issue

What changed: Path resolution logic in lua/opencode/events/permissions/edits.lua:23-40 — resolves filepath by trying absolute (:p) first, then $HOME + filepath as fallback, with a guard that notifies the user if neither works.

Correctness

The logic is sound. fnamemodify(":p") returns already-absolute paths as-is. The HOME fallback handles relative paths. The standalone filereadable guard at line 37 correctly catches all failure cases, addressing the nested-if edge case from prior reviews.

Edge case — empty/missing diff

If event.properties.metadata.diff is nil or "", vim.split(nil, "\n") errors or writefile writes an empty patch, causing diffpatch to fail. Consider a guard:

if not diff or diff == "" then
  vim.notify("No diff content in OpenCode edit request", vim.log.levels.ERROR, { title = "opencode" })
  return
end

Nit — comment on line 57

--  FIX: Errors in diff occur due to opencode's trimDiff function

This reads like something actionable still needs fixing, but per the PR discussion this is a server-side behavior the plugin can't work around. Consider -- NOTE: instead.

Style

Follows project conventions (2-space indent, double quotes, no call parens). Clean.

PR health

Single file, focused change (+21/-2). Description is light on the "why" but the conversation fills the gap. No unrelated changes.

Suggestions are purely advisory — the human can decide what to do with them.

New%20session%20-%202026-06-12T19%3A40%3A30.109Z
opencode session  |  github run

@nickjvandyke nickjvandyke merged commit d0ad9f3 into nickjvandyke:main Jun 12, 2026
2 checks passed
@nickjvandyke

Copy link
Copy Markdown
Owner

Thank you for investigating this tricky issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants